lndclient: block until chain notifier is ready #255

starius · 2025-11-10T20:41:09Z

LND commit lightningnetwork/lnd@c6f458e (v0.20.0-rc3) moved ChainNotifier startup later in the lifecycle, so RegisterBlockEpochNtfn callers now see "chain notifier RPC is still in the process of starting" coming from Recv().

The new BlockUntilChainNotifier config option repeatedly calls RegisterBlockEpochNtfn during startup and only proceeds once the stream yields its first block height, retrying solely when we detect the ErrChainNotifierServerNotActive condition introduced by the LND commit above.

The wait is guarded by a "chainrpc" build-tag check, so deployments without the ChainNotifier service fail fast when the option is requested.

Pull Request Checklist

PR is opened against correct version branch.
Version compatibility matrix in the README and minimal required version
in lnd_services.go are updated.
Update macaroon_recipes.go if your PR adds a new method that is called
differently than the RPC method it invokes.

Include lightninglabs/lndclient#255 TODO: remove replace

Use lightninglabs/lndclient#255

Include lightninglabs/lndclient#255 TODO: remove replace

Use lightninglabs/lndclient#255

mohamedawnallah

Thanks, LGTM starius/loop@26ee295!

LND commit lightningnetwork/lnd@c6f458e (v0.20.0-rc3) moved ChainNotifier startup later in the lifecycle

By the way, this commit (lightningnetwork/lnd@c6f458e) in LND v0.20.0-rc3 moved ChainNotifier startup back to its original location from LND v0.18.5. This location was changed in the first release candidate for LND v0.19.0, and this commit reverts it back to the v0.18.5 proper behavior

mohamedawnallah · 2025-11-10T21:06:24Z

lnd_services.go


+	// If requested, wait until the chain notifier RPC is ready before we
+	// return. This ensures sub-servers relying on the notifier don't fail
+	// during startup.


Include lightninglabs/lndclient#255 TODO: remove replace

Use lightninglabs/lndclient#255

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

ziggie1984

Interesting approach had some questions.

ziggie1984 · 2025-11-12T13:16:56Z

lnd_services.go

+	// return. This ensures sub-servers relying on the notifier don't fail
+	// during startup.
+	if cfg.BlockUntilChainNotifier {
+		if !hasBuildTag(services.Version, "chainrpc") {


is this referring to the tags of lndclient or is this related to the tags sof LND which the client is connected to ?

I wonder if we really need to rely on build tags, will the waitForChainNotifier func not just error when there is no notifier ?

I removed this check. chainrpc is a requirement, according to lndclient/README.md

ziggie1984 · 2025-11-14T08:16:59Z

lnd_services.go

+	timeout, pollInterval time.Duration) error {
+
+	mainCtx := ctx
+	if mainCtx == nil {


haven't seen this pattern so far, why don't we trust the caller to provide a non-nil context ?

Removed it. AI glitch

ziggie1984 · 2025-11-14T08:41:19Z

lnd_services.go

+
+	for {
+		// Make new RegisterBlockEpochNtfn call.
+		subCtx, cancel := context.WithTimeout(mainCtx, timeout)


why not use:
defer cancel() it is way cleaner ?

I think the aim here is to ensure the subCtx is cancelled before we enter a new for loop iteration in the polling case (after case err := <-errChan: is triggered). But since non of the variables returned from the register function call is no longer referenced, I think garbage collection should make that isn't really an issue we need to worry about right?

I reworked it to make a lambda function attempt which makes one attempt. It creates subCtx and cancels it in a defer statement.

ziggie1984 · 2025-11-14T08:42:22Z

lnd_services.go

+
+		// Wait for block height notification, which indicates success.
+		select {
+		case <-mainCtx.Done():


I think we need to wait for the subCtx here?

Yeah I think since a timeout of the subCtx won't throw an error that returns true for the isChainNotifierStartingErr, we might as return in the subCtx is done here.

Good idea! Changed to waiting for subCtx here.

If subCtr expires, it means, we haven't got the first block notification in 30s (default setting of cfg.RPCTimeout). RegisterBlockEpochNtfn must return the current block height immediately. So this is a real error.

ziggie1984 · 2025-11-14T08:42:41Z

lnd_services.go

+		// Wait for block height notification, which indicates success.
+		select {
+		case <-mainCtx.Done():
+			cancel()


can be removed when we. use defer cancel() above

Removed the call, used defer statement in attempt lambda.

ziggie1984 · 2025-11-14T08:43:25Z

lnd_services.go

 }

+// chainNotifierStartupMessage matches the error string returned by lnd
+// v0.20.0-rc3+ when a ChainNotifier RPC is invoked before the sub-server


I wonder why this was not a problem before because pre 19 it was as it is now ?

I don't know. This issue was only detected by LiT itest, not in Loop and anywhere else. My guess is that when pre-19 was around, there was no this LiT itest or something was different and it didn't fire.

ziggie1984 · 2025-11-14T08:46:56Z

/gemini review

ViktorT-11

Generally looks good 🚀! Thanks for this 🙏! As @ziggie1984 has already commented, I think you can potentially clean up the context handling quite a bit :)

ViktorT-11 · 2025-11-14T13:23:47Z

lnd_services.go

+
+		// Wait for block height notification, which indicates success.
+		select {
+		case <-mainCtx.Done():


Yeah I think since a timeout of the subCtx won't throw an error that returns true for the isChainNotifierStartingErr, we might as return in the subCtx is done here.

ViktorT-11 · 2025-11-14T13:26:20Z

lnd_services.go

+			if isChainNotifierStartingErr(err) {
+				select {
+				case <-time.After(pollInterval):
+					continue


Without having tested this myself on the first iteration, I just want to make sure that something is logged here or during the register call. Just so that the user sees that there's still some progress and that the application isn't frozen (similar to how the waitForChainSync function triggers logging of every GetInfo poll attempt.).

I added logging. Each attempt and its outcome is logged with level INFO.

ViktorT-11 · 2025-11-14T13:27:33Z

lnd_services.go

+
+	for {
+		// Make new RegisterBlockEpochNtfn call.
+		subCtx, cancel := context.WithTimeout(mainCtx, timeout)


I think the aim here is to ensure the subCtx is cancelled before we enter a new for loop iteration in the polling case (after case err := <-errChan: is triggered). But since non of the variables returned from the register function call is no longer referenced, I think garbage collection should make that isn't really an issue we need to worry about right?

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

starius · 2025-11-14T15:43:18Z

I updated lightninglabs/lightning-terminal#1170 to use the most recent of this PR (60c2e6e) to check if it still fixes the itests.

starius · 2025-11-14T16:16:24Z

Added logging

ViktorT-11

Thanks a lot 🔥, uTACK LGTM 🙏!

lnd_services.go

ziggie1984

LGTM

LND commit c6f458e478f9 (v0.20.0-rc3) moved ChainNotifier startup later in the lifecycle, so RegisterBlockEpochNtfn callers now see "chain notifier RPC is still in the process of starting" coming from Recv(). The new BlockUntilChainNotifier config option repeatedly calls RegisterBlockEpochNtfn during startup and only proceeds once the stream yields its first block height, retrying solely when we detect the ErrChainNotifierServerNotActive condition introduced by the LND commit above.

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

starius · 2025-11-14T17:42:59Z

Tagged as v0.20.0-6

Include lightninglabs/lndclient#255 lndclient: block until chain notifier is ready

starius requested review from ViktorT-11, mohamedawnallah and ziggie1984 November 10, 2025 20:41

starius force-pushed the block-until-chain-notifier-is-ready branch from e581dc3 to be4fd2c Compare November 10, 2025 20:43

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

go.mod: update lndclient

6835de3

Include lightninglabs/lndclient#255 TODO: remove replace

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

loopd: wait for chain notifier

d587e4c

Use lightninglabs/lndclient#255

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

go.mod: update lndclient

342ec41

Include lightninglabs/lndclient#255 TODO: remove replace

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

loopd: wait for chain notifier

26ee295

Use lightninglabs/lndclient#255

mohamedawnallah approved these changes Nov 10, 2025

View reviewed changes

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

go.mod: update lndclient

f4f0f06

Include lightninglabs/lndclient#255 TODO: remove replace

starius added a commit to starius/loop that referenced this pull request Nov 10, 2025

loopd: wait for chain notifier

0b53957

Use lightninglabs/lndclient#255

starius added a commit to starius/lightning-terminal that referenced this pull request Nov 10, 2025

[test] bump lndclient and loop

79701bd

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

This was referenced Nov 10, 2025

update lndclient and enable waiting until chain notifier is ready lightninglabs/lightning-terminal#1170

Open

chainrpc: retry notifier RPCs during startup lag #253

Draft

ziggie1984 reviewed Nov 14, 2025

View reviewed changes

ViktorT-11 mentioned this pull request Nov 14, 2025

WIP: go.mod: bump lnd, loop, taproot-assets, taprpc, lndclient lightninglabs/lightning-terminal#1173

Open

ViktorT-11 reviewed Nov 14, 2025

View reviewed changes

starius force-pushed the block-until-chain-notifier-is-ready branch from be4fd2c to 60c2e6e Compare November 14, 2025 15:37

starius added a commit to starius/lightning-terminal that referenced this pull request Nov 14, 2025

[test] bump lndclient and loop

c087d57

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

starius force-pushed the block-until-chain-notifier-is-ready branch from 60c2e6e to efe0aa0 Compare November 14, 2025 16:15

starius requested review from ViktorT-11 and ziggie1984 November 14, 2025 16:16

ViktorT-11 approved these changes Nov 14, 2025

View reviewed changes

ViktorT-11 reviewed Nov 14, 2025

View reviewed changes

lnd_services.go Show resolved Hide resolved

ziggie1984 approved these changes Nov 14, 2025

View reviewed changes

starius force-pushed the block-until-chain-notifier-is-ready branch from 04a63fd to d4b697c Compare November 14, 2025 17:00

starius added a commit to starius/lightning-terminal that referenced this pull request Nov 14, 2025

[test] bump lndclient and loop

19e24c1

Include lightninglabs/lndclient#255 Include lightninglabs/loop#1039 wait for chain notifier

starius merged commit eaf0b75 into lightninglabs:lnd-20-0 Nov 14, 2025
1 check passed

starius deleted the block-until-chain-notifier-is-ready branch November 14, 2025 17:40

starius added a commit to starius/lightning-terminal that referenced this pull request Nov 14, 2025

go.mod: bump lndclient to v0.20.0-6

1d43564

Include lightninglabs/lndclient#255 lndclient: block until chain notifier is ready

lndclient: block until chain notifier is ready #255

lndclient: block until chain notifier is ready #255

Conversation

starius commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

mohamedawnallah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziggie1984 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziggie1984 commented Nov 14, 2025

Uh oh!

ViktorT-11 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

starius commented Nov 14, 2025

Uh oh!

starius commented Nov 14, 2025

Uh oh!

ViktorT-11 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ziggie1984 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

starius commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

starius commented Nov 10, 2025 •

edited

Loading

mohamedawnallah left a comment •

edited

Loading

ziggie1984 left a comment •

edited

Loading